Skip to content

video: add stm32mp135f gc2145 camera board shield & enable dcmipp / st-mipid02 #91837

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 4 commits into from
Jun 27, 2025
Merged

video: add stm32mp135f gc2145 camera board shield & enable dcmipp / st-mipid02 #91837

merged 4 commits into from
Jun 27, 2025

Conversation

avolmat-st
Copy link
Collaborator

@avolmat-st avolmat-st commented Jun 18, 2025 •
edited
Loading

This PR adds the shield for the GC2145 camera module available with the stm32mp135f-dk

This PR depends on PR #91878 for allowing crop in order to properly display the image

@avolmat-st avolmat-st requested review from erwango and josuah June 18, 2025 12:26
@avolmat-st avolmat-st force-pushed the video_gc2145_mp13_shield branch from d4ae1d2 to 3223894 Compare June 20, 2025 12:28
@avolmat-st avolmat-st marked this pull request as ready for review June 20, 2025 12:29
@github-actions github-actions bot added platform: STM32 ST Micro STM32 area: GPIO area: Video Video subsystem area: Shields Shields (add-on boards) area: Samples Samples labels Jun 20, 2025
@erwango erwango added this to the v4.2.0 milestone Jun 20, 2025
erwango
erwango requested changes Jun 20, 2025
View reviewed changes
Copy link
Member

@erwango erwango left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Otherwise, looks ok.

boards/st/stm32mp135f_dk/stm32mp135f_dk_defconfig Outdated
@@ -14,3 +14,6 @@ CONFIG_CONSOLE=y

# UART console (overrides remote proc console)
CONFIG_UART_CONSOLE=y

# Ensure VIDEO is initialized after pinctrl due to IO-Expander
CONFIG_VIDEO_INIT_PRIORITY=75
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Put in board's kconfig.defconfig depeding on VIDEO

@avolmat-st avolmat-st force-pushed the video_gc2145_mp13_shield branch from 3223894 to 7647177 Compare June 20, 2025 14:55
@avolmat-st
Copy link
Collaborator Author

Added Kconfig.defconfig in stm32mp135f_dk board as per @erwango comment.

erwango
erwango previously approved these changes Jun 20, 2025
View reviewed changes
josuah
josuah reviewed Jun 20, 2025
View reviewed changes
Copy link
Collaborator

@josuah josuah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM thank you!

Just waiting to get your feedback on init priorities to confirm this was intentional, but I stand ready to give a quick review cycle to approve after that is clarified.

@avolmat-st avolmat-st force-pushed the video_gc2145_mp13_shield branch from d09a755 to 47fe3a0 Compare June 26, 2025 20:21
@avolmat-st avolmat-st requested review from kartben and josuah June 26, 2025 20:22
josuah
josuah previously approved these changes Jun 26, 2025
View reviewed changes
Copy link
Collaborator

@josuah josuah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Glad to see more shields being used, that reduces efforts of manually wiring things up in devicetree.

boards/st/stm32mp135f_dk/stm32mp135f_dk.dts
};

port@1 {
csi_capture_port: endpoint { };
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apparently dtc does not complain that the remote-endpoint-label required property is absent?
Then no need to add the stub remote-endpoint-label = ""; I suppose!

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not on that one since there is a compatible in the stm32mp135.dtsi

Copy link
Collaborator

@ngphibang ngphibang Jun 27, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see where the compatible = "st,stm32-dcmipp-pipe"; yaml file is. But I think inside that .yaml, we should still include: video-interfaces.yaml because this is an endpoint no ? If not, it seems we made an exception for this endpoint

Copy link
Collaborator

@ngphibang ngphibang Jun 27, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's how each pipe output is being described, based on the very first discussion we had at the beginning when I started implementing that.

Thanks for reminding me. Yes, I remember we have some constraints to support multiple pipes in the DCMIPP and come up to this solution but didn't pay attention to this particular trade-off.

This is needed but not a real endpoint (no yaml, no remote-endpoint to interface with, no bus-type). Maybe we can try to see if we can have a nicer solution in the future. The current implementation is OK to me. Thanks.

erwango
erwango previously approved these changes Jun 27, 2025
View reviewed changes
@kartben kartben dismissed stale reviews from erwango and josuah via e07e6e9 June 27, 2025 07:37
@kartben kartben force-pushed the video_gc2145_mp13_shield branch from 47fe3a0 to e07e6e9 Compare June 27, 2025 07:37
kartben
kartben previously approved these changes Jun 27, 2025
View reviewed changes
@kartben
Copy link
Collaborator

kartben commented Jun 27, 2025

adding 100KB to the repo for such a tiny image was really overkill :) updated to webp

@kartben kartben requested review from erwango and josuah June 27, 2025 07:38
@kartben
Copy link
Collaborator

kartben commented Jun 27, 2025

and of course I messed up... on it

@kartben kartben force-pushed the video_gc2145_mp13_shield branch from e07e6e9 to 295cd30 Compare June 27, 2025 07:46
kartben
kartben previously approved these changes Jun 27, 2025
View reviewed changes
ngphibang
ngphibang requested changes Jun 27, 2025
View reviewed changes
Copy link
Collaborator

@ngphibang ngphibang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added change-request to wait for more discussion about my question on csi_capture_port: endpoint { }; maybe I am wrong, will remove it as soon as more info is provided

Alain Volmat added 2 commits June 27, 2025 10:19
@kartben
shields: st_mb1897_cam: addition of MB1897 camera board
cfb7df0 
Addition of a shield st_mb1897_cam embedding a GC2145 CSI
sensor which can be connected to the STM32MP135F-DK via a 15pins FFC.

Signed-off-by: Alain Volmat <alain.volmat@foss.st.com>
@kartben
samples: video: capture: add a stm32mp135f_dk config
8a8d5b9 
Add a configuration file for the stm32mp135f_dk board.

Signed-off-by: Alain Volmat <alain.volmat@foss.st.com>
@kartben kartben force-pushed the video_gc2145_mp13_shield branch from 295cd30 to 8a8d5b9 Compare June 27, 2025 08:19
@avolmat-st
Copy link
Collaborator Author

Added change-request to wait for more discussion about my question on csi_capture_port: endpoint { }; maybe I am wrong, will remove it as soon as more info is provided

This is part of the stm32mp135.dtsi in the description of the dcmipp node:

                dcmipp: dcmipp@5a000000 {
                        compatible = "st,stm32mp13-dcmipp", "st,stm32-dcmipp";
                        reg = <0x5a000000 0x400>;
                        interrupts = <GIC_SPI 79 IRQ_TYPE_LEVEL IRQ_DEFAULT_PRIORITY>;
                        clocks = <&rcc STM32_CLOCK(APB4, 1)>,
                                 <&rcc STM32_SRC_PLL2_Q DCMIPP_SEL(1)>;
                        clock-names = "dcmipp", "dcmipp-ker";
                        resets = <&rctl STM32_RESET(APB4, 1)>;
                        status = "disabled";

                        ports {
                                #address-cells = <1>;
                                #size-cells = <0>;

                                port@0 {
                                        reg = <0>;

                                        endpoint {
                                                remote-endpoint-label = "";
                                                bus-type = <VIDEO_BUS_TYPE_PARALLEL>;
                                        };
                                };

                                port@1 {
                                        reg = <1>;

                                        dcmipp_pipe_dump: endpoint {
                                                compatible = "st,stm32-dcmipp-pipe";
                                        };
                                };
                        };
                };

That's how each pipe output is being described, based on the very first discussion we had at the beginning when I started implementing that.

@avolmat-st
Copy link
Collaborator Author

adding 100KB to the repo for such a tiny image was really overkill :) updated to webp

Thanks @kartben, sorry I didn't noticed ;(

@sonarqubecloud SonarQubeCloud
Copy link

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

@kartben kartben merged commit e4348ad into zephyrproject-rtos:main Jun 27, 2025
26 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kartben kartben kartben approved these changes

@ngphibang ngphibang ngphibang approved these changes

@josuah josuah josuah approved these changes

@avisconti avisconti Awaiting requested review from avisconti

@djiatsaf-st djiatsaf-st Awaiting requested review from djiatsaf-st

@etienne-lms etienne-lms Awaiting requested review from etienne-lms

@FRASTM FRASTM Awaiting requested review from FRASTM

@gautierg-st gautierg-st Awaiting requested review from gautierg-st

@GeorgeCGV GeorgeCGV Awaiting requested review from GeorgeCGV

@henrikbrixandersen henrikbrixandersen Awaiting requested review from henrikbrixandersen

@jfischer-no jfischer-no Awaiting requested review from jfischer-no

@marwaiehm-st marwaiehm-st Awaiting requested review from marwaiehm-st

@mathieuchopstm mathieuchopstm Awaiting requested review from mathieuchopstm

@mnkp mnkp Awaiting requested review from mnkp

@erwango erwango Awaiting requested review from erwango

Assignees

@kartben kartben

Labels
area: GPIO area: Samples Samples area: Shields Shields (add-on boards) area: Video Video subsystem platform: STM32 ST Micro STM32
Projects
None yet
Milestone
v4.2.0
Development

Successfully merging this pull request may close these issues.
5 participants
@avolmat-st @kartben @ngphibang @erwango @josuah